[Runtime field editor] Component integration test for preview #106410
Conversation
sebelga
force-pushed
the
runtime-field-editor/component-integration-tests
branch
from
e794b0a to
04084c7
Compare
| } | ||
| }, [currentDocument, isCustomId]); | ||
|
|
||
| useDebounce( |
There was a problem hiding this comment.
I have moved the logic to fetch the document inside the preview context file to have a single place to look at for fetching document. This also allows us to keep the state when the doc navigation is unmounted (for example when showing the empty prompt)
sebelga
force-pushed
the
runtime-field-editor/component-integration-tests
branch
from
04084c7 to
937f2fa
Compare
| previewCount === 0; | ||
| : name === null && format === null | ||
| ? true | ||
| : false; |
There was a problem hiding this comment.
I wonder if this expression could be made more readable by splitting into several if/else statement with isEmptyPromptVisible being false by default? I find multiple ternary operators very complex to understand.
There was a problem hiding this comment.
You right, your suggestion will make it easier to follow. I'll make the change 👍
There was a problem hiding this comment.
Hi @sebelga, great work! I think adding so many tests makes this feature way more robust and stable!
I followed the tests and tried out everything locally. The preview works as expected and I haven't noticed any bugs.
I'd like to comment on some UX that might be improved in a follow up work and are not blockers for this PR:
- When the error is too long, it goes out of the callout, I like to use EUI's utility classes to fix this (
eui-textBreakWordoreui-textBreakAll)
-
Pinned fields are not preserved between a custom document ID and loading documents from the cluster
-
When the document changes, the field's preview is updating (which is indicated on the top of the flyout), but there is no indication for that at the field itself. It might be confusing when the value just suddenly changes, especially when other fields have already been loaded from the document (I tested this on 'Slow 3G')
|
Thanks for the review @yuliacech! You made some great UX enhancement suggestions. I added them in the backlog and will address them as a separate chunk of work. 👍 |
|
|
|
@elasticmachine merge upstream |
…time-field-editor/component-integration-tests
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
|
Thanks for the review @mattkime !
Good catch, I removed the TODO as it has been covered. 👍 |
sebelga
merged commit 3954533
into
elastic:runtime-field-editor/preview-field-workstream
|
@sebelga Could you please add labels to this PR, too? |
sebelga
added
Project:RuntimeFields
Team:Kibana Management
|
Pinging @elastic/kibana-stack-management (Team:Stack Management) |
This PR adds the component integration tests for the preview panel inside the runtime field editor. While adding the tests I found a few issues around timings and inconsistencies when updating some states (I found them while doing QA with throttling on "Slow 3G"). Specially when switching between cluster data and loading a custom doc ID.
I am confident that the code is more robust now and the tests should give us a very high level of confidence against future regressions. 👍
Preview for concrete fields
I've also enabled the preview for concrete field when the "Set format" is turned on.